Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add record keeping for debugging #628

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add record keeping for debugging #628

wants to merge 1 commit into from
+262 −17

Conversation

Goirad
Copy link
Member

@Goirad Goirad commented Jul 22, 2024

This PR has the enclave runner keep track about all of the usercalls that enclaves do over their lifetimes. This includes:

  • The total number of each async usercall done
  • How many of each sync usercall each enclave TCS has done
  • For waits and sends, additional grouping by signal type

The intended integration path is for consumers of this library to set up signal handling or some other mechanism to trigger collecting and logging the stats. The included formatting looks like this (from a web server enclave using async usercalls):

Async usercall counts: read: 41, write: 33, accept_stream: 3, connect_stream: 11, insecure_time: 9
Sync usercall count mappings:
Address: 0x00007fc08e2af000
    Sync Totals: launch_thread: 1, wait: 80, send: 77
    Wait Totals: RETURNQ_NOT_EMPTY: WAIT_NO: 0 WAIT_INDEFINITE: 79 OTHER: 0
                 UNPARK: WAIT_NO: 0 WAIT_INDEFINITE: 1 OTHER: 0
    Send Totals: UNPARK: 77
Address: 0x00007fc08b276000
    Sync Totals: bind_stream: 3, launch_thread: 1, wait: 40, send: 2, alloc: 12, free: 12
    Wait Totals: UNPARK: WAIT_NO: 0 WAIT_INDEFINITE: 7 OTHER: 33
    Send Totals: UNPARK: 2
Address: 0x00007fc08823d000
    Sync Totals: launch_thread: 1, wait: 3, send: 2, alloc: 5
    Wait Totals: UNPARK: WAIT_NO: 0 WAIT_INDEFINITE: 3 OTHER: 0
    Send Totals: UNPARK: 2
Address: 0x00007fc08722a000
    Sync Totals: write: 6, close: 2, launch_thread: 8, wait: 13, send: 6, alloc: 19, free: 23, async_queues: 1
    Wait Totals: UNPARK: WAIT_NO: 0 WAIT_INDEFINITE: 13 OTHER: 0
    Send Totals: UNPARK: 6
Address: 0x00007fc08d29c000
    Sync Totals: close: 8, launch_thread: 1, wait: 36, send: 5, alloc: 40, free: 40
    Wait Totals: UNPARK: WAIT_NO: 0 WAIT_INDEFINITE: 36 OTHER: 0
    Send Totals: UNPARK: 5

This is just an initial pass, I definitely need to add more robust and/or elegant logic related to the absolute number of usercalls and stringifying them.

@Goirad Goirad force-pushed the dario/add-stats branch from b52ff9c to c5758a9 Compare July 23, 2024 18:03
@Goirad
Add record keeping for debugging
0aa3d77 
This PR has the enclave runner keep track about all of the usercalls
that enclaves do over their lifetimes. This includes:
- The total number of each async usercall done
- How many of each sync usercall each enclave TCS has done
- For `wait`s and `send`s, additional grouping by signal type
@Goirad Goirad force-pushed the dario/add-stats branch from c5758a9 to 0aa3d77 Compare July 23, 2024 18:04
jethrogb
jethrogb reviewed Aug 2, 2024
View reviewed changes
Copy link
Member

@jethrogb jethrogb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of aggregating everything already, wouldn't it be better to have some strace-style logging of every call and then do post-processing later?

intel-sgx/enclave-runner/src/usercalls/mod.rs
Comment on lines +60 to +62
const USERCALL_QUEUE_SIZE: usize = 32768;
const RETURN_QUEUE_SIZE: usize = 32768;
const CANCEL_QUEUE_SIZE: usize = USERCALL_QUEUE_SIZE;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated change?

@Goirad
Copy link
Member Author

Goirad commented Aug 2, 2024

Instead of aggregating everything already, wouldn't it be better to have some strace-style logging of every call and then do post-processing later?

That seems a lot harder, because the application's stdout/stderr is intermingled with the enclave runner's stdout/stderr. Also, the overhead of printing that is non-trivial, and would probably have to be opted into, while this atomic based aggregation is essentially zero-cost.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jethrogb jethrogb jethrogb left review comments

@s-arash s-arash Awaiting requested review from s-arash

@raoulstrackx raoulstrackx Awaiting requested review from raoulstrackx

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Goirad @jethrogb